fix(docker): PTY container ref-count, exec error propagation, Docker-wizard i18n#99
Conversation
…te i18n Follow-up fixes from the PR #96/#97 review (CodeRabbit + adversarial verification). - HIGH: a Docker terminal held no container reference, so an env switch that released the engine's last ref would stop the container out from under a live terminal. The PTY backend now acquires its own ref via AcquireDockerContainer for its lifetime and releases it on Close (a no-op for a container the user already had running). (internal/web/pty.go, internal/web/server.go) - MED: `docker exec` discarded the StdCopy stream error and swallowed ContainerExecInspect errors, so a broken/partial exec could be reported as success and persisted by the edit tools. Both are now surfaced. (internal/tools/docker_exec.go) - i18n: the 8 Docker-wizard keys existed only in en.ts; added them to zh-Hans, zh-Hant, ja, ko so non-English users no longer see English fallback text. Deferred to a separate PR: config read-modify-write lock standardization (s.mu vs s.cfgMu). Verified as non-issues / acknowledged: the "{-input swallowed" claim (input arrives as binary frames), the docker/docker CVEs (daemon-side, client-only usage), pty-cancel, and the pending-conn shutdown sweep. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 4 minutes. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Follow-up fixes for the merged Docker-workspace PR (#97), from the CodeRabbit review cross-checked with an adversarial verification pass. Scope B: the confirmed High + Medium + i18n items; the config-lock standardization is intentionally left to its own PR.
Fixes
switch_envreleasing the engine's last ref wouldContainerStopa container we started while a live terminal was still attached, killing the user's shell. The PTY backend now acquires its own ref viaAcquireDockerContainerfor its lifetime and releases it onClose(). For a container the user already had running this takes no ref andClose()is a no-op (we still never stop the user's own container).internal/web/pty.go,internal/web/server.go.DockerExecutor.run()discarded thestdcopy.StdCopystream error and swallowedContainerExecInspecterrors (ierr == nil && …), so a broken/partial exec could be reported as success — and the edit tools (read-modify-write) could then persist truncated content. Both errors are now surfaced.internal/tools/docker_exec.go.container,dockerConnection,dockerDesc,selectContainer,noContainers,refresh,running,stopped) existed only inen.ts; added tozh-Hans,zh-Hant,ja,ko(parity verified across all 5 locales).Deferred / verified non-issues
s.muvss.cfgMu) across remote/channel/MCP handlers — real, but repo-wide with deadlock-ordering care needed.{is swallowed" (user input arrives as binary frames; the JSON check only runs on resize TextMessages);docker/dockerdeprecation CVEs (daemon-side, this code is client-only).Verification
go build ./...,go vet,go test ./internal/web ./internal/tools,vue-tsctype-check,oxlint,eslint,vite build— all green.🤖 Generated with Claude Code